Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ntuple(n, identity) becomes tuple(collect(1:n)...) #11

Merged
merged 1 commit into from
Jul 15, 2015
Merged

ntuple(n, identity) becomes tuple(collect(1:n)...) #11

merged 1 commit into from
Jul 15, 2015

Conversation

lostanlen
Copy link
Contributor

This came up as a fix to the deprecation warning of ntuple(n::Integer, f::Function) — see JuliaLang/julia#11486, in favor of ntuple(f, n). But actually, in the special case of f being the identity (i.e. we merely want to build (1, 2, .... n)), calling tuple(collect(1:n)...) has the same result and runs always faster on my MacBook. It allocates 2x-4x less memory and runs 1.5x-10x faster. Also, the LLVM code has 2 lines instead of 20.

What do you think ? Is there are reason why ntuple(identity, n) should be preferred over tuple(collect(1:n)...)?

This came up as a fix to the deprecation warning of ```ntuple(n::Integer, f::Function)``` — see JuliaLang/julia#11486, in favor of ```ntuple(f, n)```. But actually, in the special case of ```f``` being the identity (i.e. we merely want to build ```(1, 2, .... n)```), calling ```tuple(collect(1:n)...)``` has the same result and runs always faster on my MacBook. It allocates 2x-4x less memory and runs 1.5x-10x faster. Also, the LLVM code has 2 lines instead of 20.
timholy added a commit that referenced this pull request Jul 15, 2015
ntuple(n, identity) becomes tuple(collect(1:n)...)
@timholy timholy merged commit 45ca2dc into JuliaAttic:master Jul 15, 2015
@timholy
Copy link
Contributor

timholy commented Jul 15, 2015

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants